- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 1.4k
 
ENH: adds annotation filtering to raw figures #13425
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: adds annotation filtering to raw figures #13425
Conversation
| 
           Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴  | 
    
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable. Can you add a test? Or even better, modify an existing test (to save on testing time) and make sure that the set of annotations is indeed limited compared to the default show-all. And yes you can add doc/changes/devel/13425.newfeature.rst with the :newcontrib: role and a new entry in doc/changes/names.inc!
| color=None, | ||
| bad_color="lightgray", | ||
| event_color="cyan", | ||
| annotation_regex=".*", | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| annotation_regex=".*", | |
| *, | |
| annotation_regex=".*", | 
| color=None, | ||
| bad_color="lightgray", | ||
| event_color="cyan", | ||
| annotation_regex=".*", | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're going to add it here (which is reasonable) we need to make everything kwarg-only (which we've been doing to functions bit-by-bit)
| annotation_regex=".*", | |
| *, | |
| annotation_regex=".*", | 
        
          
                mne/io/base.py
              
                Outdated
          
        
      | color, | ||
| bad_color, | ||
| event_color, | ||
| annotation_regex, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and everything afterward should be passed as keywords like annotation_regex=annotation_regex,
| 
           Thank you very much for the fast response! I have applied the changes you have mentioned and added some sample tests. I have added the tests to the already existing  Let me know if I should make any additional changes to the documentation or if I should convert this to a real pull request. Thanks! Edit:  | 
    
          
 Sure feel free to add it there! Would be nice for user consistency I think  | 
    
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…13350) Signed-off-by: Emmanuel Ferdman <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Larson <[email protected]>
…ols#13428) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Larson <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Larson <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Larson <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Larson <[email protected]>
Co-authored-by: Eric Larson <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Daniel McCloy <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…#13442) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Eric Larson <[email protected]>
for more information, see https://pre-commit.ci
…ython into annotation_regex
| 
           Sorry, just realised I merged instead of rebasing. Should I remake the request?  | 
    
          
 At the end of the day we squash+merge, what's important to look correct is the "Files Changed" https://github.com/mne-tools/mne-python/pull/13425/files This looks wrong... so you can either force-push to your branch to fix it, or open a new PR, either way is fine!  | 
    
| 
           Closing and moving this pull request here: #13460  | 
    
Reference issue
Looking to solve issue #13325. Thank you to @larsoner for allowing me to go ahead with this enhancement.
What does this implement/fix?
This feature looks to add a convenience parameter in the plot function to automatically hide annotation labels that we are currently not interested in. For example, if we have a dataset that uses the ECG R peak as a response, this might make the stimulus triggers more difficult to see and slows down moving around the dataset due to additional computational cost.
To mitigate this, this pull request looks at adding a
annotation_regexparameter which takes a regex as input and includes any matching annotation label in the view and hides the rest (default.*for all annotations).The current version of this feature changes the current
fig.mne.visible_annotationsvariable from setting True to everything, to setting True to only those annotation labels that fit theannotation_regexparameter.This was the most logical place to put it as it deals with the initialization of
fig.mne.visible_annotations, works with both qt and matplotlib backends and does not change the behaviour of the actual browser when you use it.Additional information
I have set this pull request to be a draft, as I have just implemented the bare minimum to create the feature to see if where I am editing the code is implemented well. There is currently no documentation (Though this would be a quick add).
Example Code using the new feature
Example Images
All Triggers in a Matplotlib Browser
Triggers with Parameter
annotation_regex="^Stimulus"in a Matplotlib BrowserTriggers with Parameter
annotation_regex="^Stimulus"in a QT BrowserLet me know if I can supply anything else or if I am ok to add the documentation regarding the current code and change this to an actual pull request.
Thanks in advance!